New trade statistics #4611
Merged
New trade statistics #4611
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add `boolean contains(Capability capability)` method
Set address prefix to empty bytes in case we know that peer has capability (updated version) Batch process mailbox messages in a thread. Refactor handling of mailbox messages
Improve logs
…thod Return early
Make capabilities final If capability changes we would have had duplicate entries
… will be done in next commits Cleanups
The check for AckMessage is not needed anymore as we remove the interface from AckMessage
Impl. applyCapabilities Cleanups
Rename getOptionalPersistedPeer to findPersistedPeer Improve getConnectedReportedPeers method
Co-authored-by: sqrrm <[email protected]>
Co-authored-by: sqrrm <[email protected]>
…onnection.hasPeersNodeAddress() which does the same internally
…node from persisted peers. Remove log in DisputeAgentManager which gets called repeatedly
Rename method
…erialisation Remove final Cleanups
…not the peer will publish only.
1. We do not want that initial data request/response use old trades statistics for excluded key hashes. Thats why we return an empty map in getMap. 2. We do not read resource file as we have removed that. 3. We do not persist as we convert the existing data and re-publish as new data, or at startup we convert the old data to the new one and then delete the file.
…st 100 we use for the selection algorithm.
…table to enforce inclusion. Cleanups, renamings
…ds on my machine. Add shutdown method to TradeStatisticsConverter and call it via TradeStatisticsManager. Now as we have a reference to TradeStatisticsConverter in we don't need the hack anymore in applyInjector. Set log level for com.neemre.btcdcli4j.core.client.ClientConfigurator to error as there is an expected warn log because of the outdated version.
@ripcurl: The complaint about private constructors (using guice this is legit) should be removed IMO.
…w. This is not the case without getting PR bisq-network#4609 merges as well. We only do it for 2 weeks after planned release time as then it can be assumed that enough nodes have updated that the normal publishing will distribute the object sufficiently.
|
Fixes #3893 |
sqrrm
reviewed
Oct 9, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replaces #4599
Based on #4610 . Initial commits are from #4610. Starts at commit 00bed02
We replace the old trade statistics which a minimal version and reduce data size from 16.5 MB to 3.9 MB. The old data gets converted to the new format.
Not updated users will not see new trade statistics and old ones only if they have peers with the old version as well. As all seed nodes will run the updated version they will not provide the old statistics. So for those users it will look weird that trade volume goes to zero over time. But I think that is an acceptable price and the alternative would be higher complexity by supporting boths trade statistics still. As 1.4.0 has so many important features I guess most active users will update quickly anyway.
If 2 traders with an old version trade they will publish both the statistics and new nodes will convert them to new trade stats using the hash of the old as hash for the new and so avoiding duplicates.
If 1 of the peers has update and the other not it will be detected via capabilities. In the new version onlt the seller publishes. So if the new version is the buyer he will not publish anyway, so we have only the old version publsihed and get converted to the new version by receiving peers. If the updated user is the seller he will check the capability of the peer and if its an old verison he will no publish so that the buyer with the old version publishes. If both users have updated then only the seller publishes.
So we have only one scenario where both publish (both have not updated) and we solve that with using the hash.
At startup we convert the existing trades stats to the new version (running in a thread, takes about 4 sec). After that we delete the old trade stats. The old trade stats will be excluded from initial data requests and response.